[ADD][14.0] New module hr_holidays_validator_list #91
Conversation
sarsurgithub
force-pushed
the
14_hr_holidays_validator_list
branch
from
b18b0e2 to
d5920e7
Compare
victorvermot
force-pushed
the
14_hr_holidays_validator_list
branch
from
d5920e7 to
305e73d
Compare
victorvermot
force-pushed
the
14_hr_holidays_validator_list
branch
5 times, most recently
from
0bb47bc to
e8ae591
Compare
victorvermot
force-pushed
the
14_hr_holidays_validator_list
branch
8 times, most recently
from
a1b2c1b to
1de1cdd
Compare
victorvermot
force-pushed
the
14_hr_holidays_validator_list
branch
from
1de1cdd to
7a177b5
Compare
| or Approver (determined in settings/users).""", | ||
| ) | ||
|
|
||
| def _set_leave_manager_id_from_leave_manager_ids(self, values): |
There was a problem hiding this comment.
You should have used a stored readable only computed method on leave_manager_id with a depends on leave_manager_ids to propagate any change.
There was a problem hiding this comment.
You're right I am going to check that
There was a problem hiding this comment.
@StephaneMangin The field leave_manager_ids is already a computed stored field. Question is can we insert nicely the set of the values from the M2M nicely or not. We might want to keep leave_manager_ids value to it's current natural computation.
What we could probably do to improve the code is:
Override https://github.com/odoo/odoo/blob/17.0/addons/hr_holidays/models/hr_employee.py#L152-L159
this way:
@api.depends('parent_id', 'leave_manager_ids')
def _compute_leave_manager(self):
for employee in self:
if self.leave_manager_ids:
self.leave_manager_id = leave_manager_ids[0]
else:
super()._compute_leave_manager()| values["leave_manager_id"] = values["leave_manager_ids"][0][-1][-1] | ||
| return values | ||
|
|
||
| def _add_leave_manager_ids_in_group(self, values): |
There was a problem hiding this comment.
An onchange would have suffice to affect groups on managers.
There was a problem hiding this comment.
I disagree, we are on hr_employee, an onchange should only affect the values of the current form. Groups won't be available from here.
| values["leave_manager_id"] = values["leave_manager_ids"][0][-1][-1] | ||
| return values | ||
|
|
||
| def _add_leave_manager_ids_in_group(self, values): |
There was a problem hiding this comment.
I disagree, we are on hr_employee, an onchange should only affect the values of the current form. Groups won't be available from here.
| or Approver (determined in settings/users).""", | ||
| ) | ||
|
|
||
| def _set_leave_manager_id_from_leave_manager_ids(self, values): |
There was a problem hiding this comment.
@StephaneMangin The field leave_manager_ids is already a computed stored field. Question is can we insert nicely the set of the values from the M2M nicely or not. We might want to keep leave_manager_ids value to it's current natural computation.
What we could probably do to improve the code is:
Override https://github.com/odoo/odoo/blob/17.0/addons/hr_holidays/models/hr_employee.py#L152-L159
this way:
@api.depends('parent_id', 'leave_manager_ids')
def _compute_leave_manager(self):
for employee in self:
if self.leave_manager_ids:
self.leave_manager_id = leave_manager_ids[0]
else:
super()._compute_leave_manager()
… leave_manager_ids, for somecase that trigger unwanted fields update
|
@victorvermot you should check the CI: the tests are failing. |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
A module making replacing the current leave_manager_id field by a many2many field called leave_manager_ids. Thus allowing a user to have several leave managers.
TODO: